Skip to content

feat: redacted logging#24

Merged
hib4 merged 3 commits intodevfrom
feat/redacted-logging
May 15, 2025
Merged

feat: redacted logging#24
hib4 merged 3 commits intodevfrom
feat/redacted-logging

Conversation

@hib4
Copy link
Member

@hib4 hib4 commented May 15, 2025 •
edited by github-actions bot
Loading

PR Type

Enhancement

Description

  • Introduce helper to redact sensitive request fields

  • Define sensitiveFields list for masking in logs

  • Recursively sanitize nested objects in request body

  • Use sanitized body in logging middleware

Changes walkthrough 📝

Relevant files
Enhancement
server.ts
Add request body sanitization for logging                               

functions/src/server.ts

  • Added sanitizeBody function to mask sensitive fields
  • Defined sensitiveFields list for redaction
  • Implemented recursive sanitization of nested objects
  • Updated logging middleware to use sanitizeBody
    		• +27/-1   
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @hib4 hib4 self-assigned this May 15, 2025
    @github-actions
    Copy link

    github-actions bot commented May 15, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Array Handling

    sanitizeBody uses object spread to clone the input but does not preserve array types, which can break logging of array payloads.

     
    const sanitizeBody = (body: any): any => {
  if (!body) return body;

  const sensitiveFields = [
    "password",
    "token",
    "secret",
    "key",
    "authorization",
    "credit_card",
    "ssn",
    "social_security",
  ];
  const sanitized = { ...body };

  Object.keys(sanitized).forEach((key) => {
    if (sensitiveFields.some((field) => key.toLowerCase().includes(field))) {
      sanitized[key] = "[REDACTED]";
    } else if (typeof sanitized[key] === "object" && sanitized[key] !== null) {
      sanitized[key] = sanitizeBody(sanitized[key]);
    }
  });

  return sanitized;
};
    Field Matching

    Using key.toLowerCase().includes(field) may cause false positives (e.g., matching "monkey"), consider stricter matching or a whitelist approach.

     
    Object.keys(sanitized).forEach((key) => {
  if (sensitiveFields.some((field) => key.toLowerCase().includes(field))) {
    sanitized[key] = "[REDACTED]";
    Performance

    Recursively cloning objects on each call could impact performance for large or deeply nested payloads; consider in-place sanitization or limiting recursion depth.

     
    const sanitized = { ...body };

Object.keys(sanitized).forEach((key) => {
  if (sensitiveFields.some((field) => key.toLowerCase().includes(field))) {
    sanitized[key] = "[REDACTED]";
  } else if (typeof sanitized[key] === "object" && sanitized[key] !== null) {
    sanitized[key] = sanitizeBody(sanitized[key]);
  }
});

    @github-actions
    Copy link

    github-actions bot commented May 15, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Guard against circular references

    Add cycle detection using a WeakSet to guard against circular references and prevent
    infinite recursion when sanitizing deeply nested objects.

    functions/src/server.ts [36-37]

    -const sanitizeBody = (body: any): any => {
-  if (!body) return body;
+const sanitizeBody = (body: any, seen = new WeakSet<any>()): any => {
+  if (body === null || typeof body !== "object") return body;
+  if (seen.has(body)) return "[Circular]";
+  seen.add(body);
    Suggestion importance[1-10]: 8

    __

    Why: Introducing cycle detection with a WeakSet avoids infinite recursion on circular objects, preventing potential crashes in sanitizeBody.

    		Medium
    Handle arrays in sanitization

    Add explicit handling for arrays so that lists are properly traversed and returned
    as arrays rather than being spread into objects. This prevents unexpected type
    changes when sanitizing array payloads.

    functions/src/server.ts [36-37]

     const sanitizeBody = (body: any): any => {
-  if (!body) return body;
+  if (body === null || typeof body !== "object") return body;
+  if (Array.isArray(body)) return body.map(sanitizeBody);
    Suggestion importance[1-10]: 7

    __

    Why: Adding array support in sanitizeBody prevents improper object spreading and ensures lists are sanitized correctly, improving robustness.

    		Medium
    Refine sensitive-field matching

    Prevent over-matching by changing substring checks to match exact field names or
    common suffix patterns. This reduces false positives (e.g. fields like "monkey").

    functions/src/server.ts [52-53]

    -if (sensitiveFields.some((field) => key.toLowerCase().includes(field))) {
+if (sensitiveFields.some((field) => {
+  const k = key.toLowerCase();
+  return k === field || k.endsWith(`_${field}`);
+})) {
   sanitized[key] = "[REDACTED]";
    Suggestion importance[1-10]: 6

    __

    Why: Tightening the match to exact names or suffixes avoids false positives like monkey matching key, enhancing accuracy with modest impact.

    		Low
    Security
    Redact sensitive headers and cookies

    Apply the same redaction logic to headers and cookies before logging to avoid
    leaking sensitive tokens or session data.

    functions/src/server.ts [66-67]

     const logData = {
   method: req.method,
   path: req.path,
-  headers: req.headers,
-  cookies: req.cookies,
+  headers: sanitizeBody(req.headers),
+  cookies: sanitizeBody(req.cookies),
    Suggestion importance[1-10]: 8

    __

    Why: Applying sanitizeBody to req.headers and req.cookies prevents leaking tokens and session data in logs, which is a significant security improvement.

    		Medium

    @hib4 hib4 merged commit fd16393 into dev May 15, 2025
    1 check passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    @hib4 hib4

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    1 participant

    @hib4